Skip to content

fix(sdk-coin-flr): support FLR C->P atomic TSS signing#9086

Merged
ArunBala-Bitgo merged 1 commit into
masterfrom
CECHO-1425
Jun 23, 2026
Merged

fix(sdk-coin-flr): support FLR C->P atomic TSS signing#9086
ArunBala-Bitgo merged 1 commit into
masterfrom
CECHO-1425

Conversation

@ArunBala-Bitgo

@ArunBala-Bitgo ArunBala-Bitgo commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

FLR C-chain to P-chain export (and the matching P-chain import) fails during the MPC signing ceremony because:

  1. sdk-coin-flr does not declare isSignablePreHashed, so the MPCv2 signing flow re-hashes the Avalanche atomic signableHex with keccak256. The signableHex is already SHA-256(txBody), so the user's signature share is computed over a different digest from BitGo's, surfacing as "Failed to combine signature shares" on the send-transaction endpoint.

  2. The TSS recipient guard rejects intent types 'import' and 'importtoc' (Avalanche / Flare cross-chain atomic imports), even though those intents legitimately have no client-supplied recipients — the wallet imports its own UTXOs and the destination is the wallet itself. That surfaces as "Recipient details are required to verify this transaction before signing".

Add isSignablePreHashed to Flr mirroring the Flrp implementation, and add 'import' / 'importtoc' to NO_RECIPIENT_TX_TYPES so the pre-signing verifier no longer blocks the flow.

Ticket: CECHO-1425

FLR C-chain to P-chain export (and the matching P-chain import)
fails during the MPC signing ceremony because:

1. sdk-coin-flr does not declare isSignablePreHashed, so the MPCv2
   signing flow re-hashes the Avalanche atomic signableHex with
   keccak256. The signableHex is already SHA-256(txBody), so the
   user's signature share is computed over a different digest from
   BitGo's, surfacing as "Failed to combine signature shares" on
   the send-transaction endpoint.

2. The TSS recipient guard rejects intent types 'import' and
   'importtoc' (Avalanche / Flare cross-chain atomic imports), even
   though those intents legitimately have no client-supplied
   recipients — the wallet imports its own UTXOs and the
   destination is the wallet itself. That surfaces as "Recipient
   details are required to verify this transaction before signing".

Add isSignablePreHashed to Flr mirroring the Flrp implementation,
and add 'import' / 'importtoc' to NO_RECIPIENT_TX_TYPES so the
pre-signing verifier no longer blocks the flow.

Ticket: WAL-1586
Session-Id: ed1da302-5a6d-427c-9cc8-9226e355d0ed
Task-Id: d6c37b72-82ce-4927-b917-f88212aedf23
@ArunBala-Bitgo ArunBala-Bitgo requested review from a team as code owners June 23, 2026 06:45
@linear-code

linear-code Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

CECHO-1425

@ArunBala-Bitgo

Copy link
Copy Markdown
Contributor Author

@claude review this PR as grug

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ArunBala-Bitgo's task —— View job


Grug Review Time

Grug review PR as requested. Grug make todo list:

  • Read repo instructions (CLAUDE.md)
  • Look at all changed files
  • Check if fix make sense for problem
  • Give grug opinion on code
  • Update comment with review

Grug Brain Analysis

Grug look at code. Grug think simple thoughts. Here what grug find:

Problem 1: Missing isSignablePreHashed

File: modules/sdk-coin-flr/src/flr.ts:96-98

Grug see good fix! FLR C->P atomic transaction already has SHA-256 hash. MPC signing no need rehash with keccak256. Code simple:

isSignablePreHashed(unsignedTx: SignableTransaction): boolean {
  return isAvalancheAtomicTx(unsignedTx);
}

Grug like. Function do one thing. Return true when atomic transaction. Return false when normal EVM transaction. Simple logic. Good docstring explain why.

Problem 2: TSS recipient guard blocks import intents

File: modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts:65-66

Grug see good fix! Add 'import' and 'importtoc' to NO_RECIPIENT_TX_TYPES set. These transactions import wallet's own UTXOs. No external recipient needed. Comment explain good:

// Avalanche / Flare cross-chain atomic imports — recipients are not supplied
// by the client because the import consumes UTXOs already owned by the
// wallet; the destination address is the wallet itself.

Grug understand. When wallet import own money, no need specify recipient. Wallet already know where money go (to itself).

Tests Look Good

Files: Test files have good coverage

Grug see tests for both fixes:

  1. isSignablePreHashed tests atomic vs normal transactions
  2. NO_RECIPIENT_TX_TYPES tests include new import types
  3. Tests verify no error thrown for import intents

Grug approve test strategy. Test what break, test what fix.

Grug Concerns

Minor: Grug notice NO_RECIPIENT_TX_TYPES has both 'enableToken' and 'enabletoken' (lines 26-27). Same with 'disabletoken'. Grug wonder if this intentional or typo? But this not part of current PR, so grug not worry.

Good: Code style consistent with rest of codebase. Comments explain business logic well. Changes minimal and focused.

Grug Verdict: GOOD FIX

Problem: Two signing ceremony failures

  1. ❌ MPCv2 rehashes atomic transaction hash → signature shares no combine
  2. ❌ TSS guard blocks import intents → "Recipient details required" error

Solution:

  1. ✅ Add isSignablePreHashed to prevent rehashing
  2. ✅ Add import types to NO_RECIPIENT_TX_TYPES to allow imports

Grug think fix correct. Code simple. Tests good. Will solve both problems described in PR.

Grug give thumbs up 👍

@ArunBala-Bitgo ArunBala-Bitgo merged commit 5a0327d into master Jun 23, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants